Skip to content

feat(qwp): recycle the symbol dictionary at a threshold to lift the cardinality cap - #91

Draft
jovfer wants to merge 77 commits into
mainfrom
qwp-dict-recycle-r1
Draft

feat(qwp): recycle the symbol dictionary at a threshold to lift the cardinality cap#91
jovfer wants to merge 77 commits into
mainfrom
qwp-dict-recycle-r1

Conversation

@jovfer

@jovfer jovfer commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

The WebSocket sender no longer has a hard symbol-cardinality stop. When its global symbol dictionary reaches a configurable threshold (symbol_dict_reset_threshold, default 100 000), the sender recycles its send stack at the next provably-safe instant — all published frames acknowledged, no row in progress, no open transaction: it closes the send loop, retires the store-and-forward slot with the existing fully-drained close (emptying it), starts a fresh dictionary epoch on the same slot, rebuilds the engine through the same code path build() uses, and reconnects. The server needs no change: supported servers (10.x+) already clear per-connection dictionary state on disconnect, and this branch pins that behavior as a wire contract. Zero wire change, zero on-disk format change; all production code is client-side (~1 500 net production lines, plus ~6 500 lines of tests).

User-visible surface

Three connect-string keys / builder methods — symbol_dict_reset (default on), symbol_dict_reset_threshold, symbol_dict_reset_max_wait_millis (bounded blocking wait for a starved reset; default 0 = opportunistic-only, a positive value opts the producer into blocking); an advisory Sender.resetSymbolDictionary() (no-op on non-QWP transports; producer-thread-only, and PooledSender forwards it to the live delegate); two observability getters on QwpWebSocketSender (getSymbolDictEpoch, getSymbolDictResetStarvationTimeouts). Externally visible frame sequence numbers stay strictly monotone across recycles via an epoch-base offset applied on every public surface (flush sequence, ack watermark, drain, progress stream, error spans). The client-side dictionary cap rises 1M → 2M (now equal to the server cap, with an OSS ordering test), and the cap error now points at the reset valve. A recycle also naturally heals a mid-life full-dict degrade (the fresh engine re-derives delta capability). Senders built via the public connect(...) overloads (no rebuild factory) and senders that don't own their engine never recycle and never arm, so isResetArmed() cannot read true against a permanently-zero resets counter.

Release-note items:

  • The CursorWebSocketSendLoop public ctor and the QwpWebSocketSender master connect(...) overload gained parameters in place.
  • The per-connection counters (getTotalFramesSent, getTotalFramesReplayed, getTotalReconnectAttempts, getTotalReconnectsSucceeded, getTotalServerErrors, …) read the live cursor loop, which the recycle rebuilds, so they restart at 0 on every recycle — they are scoped "since the last recycle". Long-lived monitors that differenced them as lifetime counters should track the lifetime-scoped epoch counter (getSymbolDictEpoch) alongside, or treat a negative delta as a recycle boundary.
  • Servers released before QuestDB 10.0.0 cap the symbol dictionary at 1M, and QWP has no wire-level negotiation of the limit. Only 10.0.0+ servers are supported; note that the re-arm floor lets a default sender's dictionary grow to 1M entries per epoch, so this is not an edge case of exotic configuration.
  • getAckedFsn()'s contract is restated on the Sender interface: on a live sender it never collapses back to −1 and keeps reporting the last durable watermark across a recycle.
  • markEverConnected() is a new public method on QwpWebSocketSender (loop-side seeding of the sender-lifetime ever-connected flag).
  • symbol_dict_reset_threshold accepts 1..1,000,000 (half the protocol cap, the re-arm floor's own ceiling); symbol_dict_reset_max_wait_millis rejects values whose nanosecond conversion overflows.
  • getSymbolDictResetsPerformed() is gone — it could never differ from getSymbolDictEpoch().

Recycle robustness

The through-line: the recycle prefers abandoning and resuming over latching or re-arming.

  • Reconnect is deferred to the I/O loop. The post-swap reconnect never re-runs the initial-connect policy on the producer thread: ensureConnected() latches hasInitialConnectRun on its first completion and routes every later entry through the ASYNC branch — the loop is built and started synchronously, the socket connect happens on the I/O thread with indefinite retry, and the producer keeps buffering into the fresh epoch's slot. A default (OFF) or SYNC store-and-forward sender therefore never sees a transport error or a reconnect budget past its initial connect, matching the steady-state store-and-forward contract. The rebuilt loop is seeded with the sender-lifetime ever-connected flag (markEverConnected, read after the outgoing loop's close() has joined the I/O thread), so wasEverConnected() stays sticky and endpoint-policy failures keep their post-init retry classification across the swap.
  • Anti-thrash re-arm floor. A live symbol set larger than the threshold would otherwise re-arm on every refill, recycling (teardown + reconnect + dictionary re-ship) every ~L·ln(L/(L−T)) rows forever. Each swap raises the re-arm bar to max(threshold, 2 × size-at-swap), capped at half the protocol cap so genuinely unbounded-cardinality producers keep recycling. No new config key; the default threshold is unchanged. Red-proofed: without the floor a bounded 6-symbol set recycles 4×, with it once. The floor arithmetic itself is pinned (doubling per swap, organic re-arm refused below it, advisory request bypasses it) — the integration suites' bounded symbol sets rely on that bypass to keep recycling.
  • A deferred engine close is awaited. The fully-drained close can return with the slot flock retained when the SF worker is wedged in a syscall past SegmentManager's bounded join (a stalled disk/NFS — exactly what the deferred-close machinery exists to survive). awaitDeferredEngineClose parks until the worker's exit path confirms the release (30 s budget) instead of letting the rebuild throw SlotLockContentionException. SymbolDictRecycleDeferredCloseTest pins both branches with a wedged-worker harness and a positive parked witness; red-proofed against the pre-fix code (dies with the exact SlotLockContentionException trace). The same protocol closes an engine the rebuild cannot keep (recovered fully-acked leftovers): a wedged worker there retains the engine and resumes on the next send instead of colliding with the next rebuild, and close() keeps reporting the flock held until the worker exits; pinned with a wedged recovered engine both riding the stall out and exhausting the await. Step 3 detaches the outgoing engine's slot-lock listener before closing it, so a release completing after the swap cannot mark the rebuilt engine's flock released; a recycle-retained engine reaches pools through the isSlotLockReleased() re-probe. The await budget is spent once per pending close: a resume while the worker is still wedged probes and rethrows without parking (pinned: the retried table() returns in well under the budget).
  • The recycle is resumable instead of terminal. The swap commits only after a fresh engine stands on the emptied slot (dictionary, epoch counters, and wiring move after the rebuild), and every transient mid-recycle failure — an interrupted producer thread at the loop close, a wedged SF worker exhausting the deferred-close budget, a momentary EMFILE at rebuild, a post-cleanup fsync warning — records a two-state resume point (CLOSE_LOOP/REBUILD) that the next send completes. The terminal latch survives for exactly one case: a rebuilt engine that recovered unacknowledged frames, which proves the outgoing close's fully-drained contract was breached. Benign recovered fully-acked leftovers (a transiently failed segment unlink) heal by closing the recovered engine — which retries the unlink by design — and rebuilding. Error passes through the recycle machinery unwrapped and unlatched. Suites pin the interrupt, rebuild-fault, await-exhaustion, heal, and breach paths, each red-proofed.
  • A rebuild-time quarantine reaches the handler installed on the sender. The rebuild factory receives the sender's current user handler (rebuild(SenderErrorHandler)), not the builder's field captured at build time, so setErrorHandler() after build() is honoured when a recycle rebuild sets a torn slot aside; the builder's factory also snapshots sf_dir/sender id at build time.
  • A failed post-swap reconnect never latches. A @TestOnly fault hook drives a real failed reconnect: one test proves ids registered mid-window survive into the flushed delta (fails with the resetSymbolDictStateForNewConnection guard reverted to the unconditional clear), the other proves the failure leaves the sender usable (fails with recycleFailure latched in the reconnect catch). Both counterfactuals re-run red against the final tree.
  • close()'s drain guard is split by true dependency (flush/commit/seal need only the engine; checkUnsurfacedError/drainOnClose need the loop). Staged rows cannot coexist with a null loop — sendRow() calls ensureConnected() before a row commits and every failure path rolls the in-progress row back — so this is hardening that keeps the guard honest for any future state, not a fix for a reachable row drop.
  • Accessors are truthful across the recycle window. getAckedFsn()/awaitAckedFsn() snapshot cursorEngine/cursorSendLoop into locals and, while the engine is null (mid-swap), report the durable watermark the recycle barrier proved instead of collapsing to −1; the epoch base is read before the engine so a torn pair can never fabricate an FSN above anything published; cursorEngine, fsnEpochBase, lastRecycleDurableFsn and the ever-connected flag are volatile for monitoring readers; the getAckedFsn javadocs state the real post-recycle sentinel behavior, including the HTTP/TCP/UDP always-−1 caveat. The torn (old base, fresh engine) clamp is pinned by a test that parks a monitor thread between the accessor's two reads across a real recycle; the null-engine window is asserted from inside the deferred-close park witness — both go red with the clamp or the durable write removed.
  • Delta-dict healing is epoch-scoped by design. The rebuilt engine re-derives delta capability rather than inheriting a one-way degrade latch: SymbolDictRecycleHealingTest pins that a degraded sender heals back into delta mode once the fault clears, and that a persistent fault degrades the fresh engine again as a normal, catchable error.
  • A rebuild that recovers unacknowledged frames latches the sender terminal (empties-the-slot contract breach); fully-acked leftovers heal. The stale slot-lock latch a pool-style re-probe can leave behind is cleared as soon as the outgoing close is confirmed, before the rebuild takes the flock (pinned through a rebuild-factory probe), and the fresh dictionary is allocated before the rebuild so the commit block genuinely cannot throw.
  • The fresh dictionary is allocated at the outgoing dictionary's size (the recovery path already did this), so an epoch's refill no longer re-climbs the rehash ladder.
  • Diagnostics use the external FSN scale. drainOnClose's two WARNs (the abandon-uncommitted-frames one and the timeout one) and its thrown message, plus the I/O loop's two replay log lines, report epoch-rebased FSNs — the same scale flushAndGetSequence() and the SenderError spans use (pinned after a real recycle).
  • Contracts the recycle touches are stated where readers look. A recycle's reconnect fires RECONNECTED/FAILED_OVER with no DISCONNECTED (now on SenderConnectionEvent.Kind and SenderConnectionListener, and pinned); a rebuild-time quarantine is delivered synchronously on the producer thread like the build-time one (SenderErrorHandler); setEngineRebuildFactory is documented and refuses a closed sender. setEngineRebuildFactory(null) stops future arming but never cancels an armed or pending recycle, and its javadoc now says so.

This update moves the recycle's default posture to fully opportunistic and closes two resume gaps found under interrupt and fault injection. symbol_dict_reset_max_wait_millis now defaults to 0: a recycle only ever runs when a table() call finds the backlog already drained, and the producer is never blocked unless a positive value opts it in — restoring the reconnect-reset design's original posture, which the earlier default had drifted from. The two park loops a recycle can enter (maybeBlockForStarvedReset()'s starvation wait, awaitDeferredEngineClose()'s deferred-close wait) now share one interrupt policy — clear the flag per park iteration, restore it on every timeout/throw exit, swallow it on success — after a carried interrupt flag was found manufacturing a false CLOSE_LOOP abandon out of an otherwise successful wait. A CLOSE_LOOP resume now re-registers the delta baseline it used to drop, republishing it as deferred dictionary chunks the fresh loop replays ahead of any data frame, and closes the outstanding chunk-commit debt on both the resume's Throwable and Error paths, so an interrupted or faulted recycle converges the same way a clean one does. resetSymbolDictionary(), the dictionary-cap error, and armIfEligible()'s javadoc now state the trigger point in one sentence each: table(...) is the only place a recycle ever runs. Eight new tests pin these: the interrupt policy under both park loops, the CLOSE_LOOP re-registration and its fallback/debt-close paths (including a crash-replay run proving the abandoned resume group survives a restart), a post-recycle endpoint-policy-rejection retry, and FSN epoch-base accumulation across two real recycles.

Design decisions

  • Opportunistic-only by default (symbol_dict_reset_max_wait_millis=0): the recycle runs when a
    table() call finds the backlog drained, per the reconnect-reset design's posture — a bounded wait on
    the hot ingestion path is not a default. A positive value is an explicit opt-in that blocks the
    producing thread at most once per armed window. The signal that a deployment never finds its drained
    instant is isResetArmed() staying true while getSymbolDictEpoch() does not advance; the planned
    escalation for that population is an asynchronous handover of the dictionary swap to the I/O loop,
    not a longer wait.
  • Recycle bounds mirror close()'s stall class, not always its blocking behavior. The swap's
    loop-close wait is literally close()'s own budget (CursorWebSocketSendLoop.close()'s 30 s
    shutdown-await) — the same method a plain close() calls. The deferred-engine-close wait is a
    separate, dedicated budget (RECYCLE_DEFERRED_CLOSE_MAX_WAIT_MILLIS, also 30 s) sized to survive the
    same disk/NFS stall class SegmentManager's bounded worker join is built to survive; unlike a plain
    close(), which spends only its own join budget and returns, leaving eventual release to a pool's
    background re-probe (isSlotLockReleased()), the recycle parks the producer on this wait so the
    rebuild never collides with a retained flock. Rows are refused only for that window, and only while
    the SF worker is genuinely wedged — a durability-failure class, the same kind of signal
    SegmentRing.checkDurability() already surfaces on the row-send path — never on a transport outage,
    and no reconnect budget applies to the producer. The deferred-close park's interrupt policy is
    code-reviewed but intentionally untested this round (the starvation wait's policy has the tests).
  • The CLOSE_LOOP resume's re-registration has its own producer-visible bound. Republishing the
    dropped delta baseline runs through the same sealAndSwapBuffer/appendBlocking path as any other
    publish, on the producer thread, bounded by sf_append_deadline_millis (default 30 s) — stated as its
    own bound, like the deferred-engine-close wait. A timeout degrades to the plain baseline drop and
    closes any chunks already on the ring best-effort (commitOrphanedDictionaryChunks, which never
    throws); if that best-effort close still leaves the group open, every subsequent flush() and
    close() re-attempts the closing commit whenever a group is open (hasDeferredMessages), so the
    residual is a retried, not permanent, state.
  • Server support floor is 10.x. QWP was experimental through the 9.x line and never released with a
    9.x server, so no compatibility shim for 9.x server behaviour exists or is argued from.
  • Sender stays single-threaded. A concurrent close() while a recycle waits is outside the
    documented contract (Sender's own thread-safety javadoc).
  • The trigger point is table(), by contract. Sender.resetSymbolDictionary(), the dictionary-cap
    error message, and armIfEligible()'s javadoc all state the trigger explicitly: the recycle runs at
    a table() call that finds the backlog acknowledged, and a caller that never starts another row never
    recycles. No per-row branch is added to the hot path.
  • Pooled senders inherit an armed (non-blocking, at the opportunistic default) recycle. An abandon
    during a give-back flush ejects the slot via discardBroken, which removes it from the pool's slot
    set so the pool can grow back into a fresh one on demand — safe, at a capacity cost until it does. The
    pooled-path recycle e2e is tracked in issue 1 million of district symbols limit #80.

Tests

13 new client suites (arming, FSN continuity, swap, memory mode, starvation, refusals, outage/orphans, catch-up skip, crash windows, healing/metrics, deferred close, slot heal/breach, reconnect fault injection — 63 tests), OSS disconnect-clear contract pin + real-client e2e (SF and memory modes) + seeded recycle×reconnect fuzz, ENT failover losslessness with recycles in flight. Verified green from a clean reactor build: client 3068/0/0/2, OSS cutlass/qwp 1397/0/0/3, ENT failover pair 3/0/0/0; the reconnect-fault counterfactuals re-run red at the final tree.

Companion PRs

🤖 Generated with Claude Code

https://claude.ai/code/session_01K2Hw6TfgX9sBB2L8oSdaWn
https://claude.ai/code/session_01NAsaMbjHq9NAENT1cJWbdg

jovfer and others added 23 commits August 17, 2026 16:13
Pull the lock/quarantine engine-construction block out of build() into
LineSenderBuilder.constructEngineOnSlotLocked()/constructEngineOnSlot(),
and expose a QwpWebSocketSender.EngineRebuildFactory seam that build()
installs on the connected sender once connect() succeeds. Pure refactor,
zero behavior change: build() keeps its wide logical-lock scope spanning
the connect loop; the standalone constructEngineOnSlot() acquires the
lock only around construction, for a later symbol-dictionary epoch
rebuild to reuse the identical construct/quarantine code path.
constructEngineOnSlotLocked() now returns a small ConstructedEngine
result (engine + whether construction itself quarantined the slot)
instead of a bare CursorSendEngine. build() seeds its own quarantined
local from that verdict, restoring the pre-refactor invariant that a
construction-time quarantine counts toward the one quarantine per
build() attempt the connect loop's retry guard allows. Without this,
a construction-time quarantine followed by an UnreplayableSlotException
from connect() would take a second quarantineTornSlot pass instead of
the original close-and-rethrow.

constructEngineOnSlot(), the public factory entry Task 5 consumes,
keeps its 8-arg signature and CursorSendEngine return type: it just
unwraps ConstructedEngine.engine and discards the quarantined verdict,
since the recycle path latches terminal on connect failure rather than
quarantining.
Adds three connect-string keys for the upcoming QWP symbol-dictionary
recycle feature: symbol_dict_reset (on/off, default on),
symbol_dict_reset_threshold (distinct-symbol count that triggers a
recycle, default 100_000, bounded by QwpConstants.MAX_SYMBOL_DICTIONARY_SIZE),
and symbol_dict_reset_max_wait_millis (upper bound on how long a
triggered recycle waits for an opportunistic window before forcing,
default 30_000, 0 means opportunistic-only).

This is config plumbing only: the three values land on QwpWebSocketSender
as resetEnabled/resetThresholdSymbols/resetMaxWaitMillis fields with
@testonly getters, following the catch_up_cap_gap_min_escalation_window_millis
knob end-to-end (ConfigSchema registry, builder methods with WS-transport
guards, both connect-string parse paths, wsConfigSnapshotForTest). Actual
recycle behavior is a follow-up task.
Adds Sender.resetSymbolDictionary(), an advisory request to start a fresh
symbol-dictionary epoch (default no-op; QwpWebSocketSender overrides it).
QwpWebSocketSender.armIfEligible() re-evaluates arming at the tail of
resetTableBuffersAfterFlush() -- the shared exit point for the plain flush,
split flush, and close-path callers -- so the recycle arms once
symbol_dict_reset is enabled and either the global dictionary reaches
symbol_dict_reset_threshold distinct entries or a caller requested a manual
reset. Arming deliberately ignores deltaDictEnabled: a sender degraded to
full self-sufficient frames still benefits from bounding dictionary growth,
and a manual request is honoured regardless of mode.

Covered by SymbolDictRecycleArmingTest: threshold crossing, symbol_dict_reset=off
never arming, the manual advisory API (both immediate and mid-batch-deferred
arming), the split-flush path sharing the same arming tail, arming in
full-dict (degraded) mode, and the no-op default on a non-WebSocket sender.
testArmsInFullDictMode previously substituted the manual
resetSymbolDictionary() advisory request for crossing
symbol_dict_reset_threshold, which never touches globalSymbolDictionary and
so left a future deltaDictEnabled-conditioned regression in threshold-based
arming undetected. Rewrite it to construct the sender through the widest
connect(List<Endpoint>, ...) overload, which accepts both a custom
symbolDictResetThresholdSymbols and the fault-injecting CursorSendEngine, and
genuinely cross the threshold (registering a, b, c) while the sender stays
degraded to full self-sufficient frames. No manual reset call remains in the
test.
- rollFsnEpochBase now throws IllegalStateException when cursorSendLoop is
  non-null: the loop's externalFsnBase is a construction-time snapshot,
  never updated on a live loop, so rolling with a loop attached would
  silently desync sender-level FSN accessors from loop-level FSN emission.
- testPreRollTargetAnswersTrueAfterRoll rolled the same already-connected
  sender that produced fsn1, so its raw engine watermark never reset and
  the pre-fix comparison (ackedFsn() == fsn1 >= fsn1) was also true --
  the test could not fail. Rebuilt around a fresh rolled sender/engine via
  the existing createRolledSender helper, matching tests 3/5/6. Tests 2, 4,
  and 7 also rolled an already-connected sender (now rejected by the new
  guard) and are rewritten the same way.
Implement the table() barrier hook and recycleForDictReset() swap: once
the symbol-dictionary recycle is armed (Task 3) and the ring is proven
drained, table() tears the cursor I/O loop and engine down, rolls the
FSN epoch base (Task 4), replaces the producer's symbol dictionary, and
rebuilds the engine via the Task 1 EngineRebuildFactory before
reconnecting -- all synchronously inside a single call.

A failed rebuild latches recycleFailure as a terminal state: every frame
existed before the swap was already proven acked, so no data is at
risk, but the sender that observed the torn-down engine/loop refuses
further use. checkRecycleFailure() covers table() and the flush-family
entry points (flush, flushAndGetSequence, drain, awaitAckedFsn) --
deliberately not close(), which must still be able to tear down a
latched sender.
Fix round 1 from review:

- maybeRecycleForDictReset() now refuses before any teardown when
  engineRebuildFactory is null (every public connect() overload leaves
  it unset -- only Sender.build() installs one) or the cursor engine
  isn't owned by this sender (setCursorEngine(engine, false)'s
  contract). Without this, a connect()-built sender with the
  default-on recycle feature armed (via resetSymbolDictionary() or a
  threshold crossing) would reach step 6, NPE against the null
  factory, and latch itself terminal for no reason.
- Step 5 also resets lastCommitBoundaryFsn -- it held a raw old-epoch
  FSN that does not survive the roll.
- checkRecycleFailure() now also guards sendRow(), closing the
  fluent-chain corner where a caller continues .symbol(...).atNow()
  against a currentTableBuffer selected before the latch, without an
  intervening table() call.
- Step 6 rewires cursorEngine.setSlotLockReleaseListener(...) on the
  rebuilt engine, restoring the pool early-wakeup notification that
  bypassing setCursorEngine had dropped.
- testPostRecycleSlotContents now asserts the post-recycle slot
  directory listing against the exact expected fresh-state file set,
  replacing a bare Files.exists(...) check that proved nothing (the
  outgoing engine had a same-named file too).
- New testConnectBuiltSenderNeverRecyclesWithoutFactory covers both
  ways such a sender can arm (manual request, threshold crossing):
  neither may recycle, throw, or stop the sender from working.
Adds SymbolDictRecycleMemoryModeTest, the sf_dir-omitted counterpart of
SymbolDictRecycleTest: threshold-triggered recycle at an empty backlog,
a content oracle proving the epoch boundary loses (and duplicates)
nothing acked, and the same recycle under initial_connect_retry=async.

All three pass unmodified against the existing recycle swap -- zero
production changes -- confirming the factory's slotPath == null arm,
CursorSendEngine's file-less close, and the table() barrier are already
mode-agnostic.
Fills the maybeBlockForStarvedReset() stub: when a symbol-dict recycle
is armed but the ring is not yet drained, opportunistically waits
(parked, awaitAckedFsn-shaped) up to symbol_dict_reset_max_wait_millis
for the outstanding acks before giving up for this armed window.
resetMaxWaitMillis<=0 disables the wait entirely; at most one blocking
wait runs per armed window (starvationWaitDoneThisArm); an open
deferred-commit group is never waited on, since the server withholds
its acks by design until the closing commit lands and this producer
thread is the only one that could ever send that commit -- blocking
there would just run out the clock every time. A timeout increments
the new symbolDictResetStarvationTimeouts counter and leaves the
recycle armed so a later drained table() call can still fire it.

Verified the deferred-commit guard is load-bearing by temporarily
removing it and confirming the test fails (blocks the full deadline
instead of returning immediately) before restoring it.
Javadoc for symbol_dict_reset_max_wait_millis (Sender.java builder
method, and the DEFAULT_.../field comments in QwpWebSocketSender.java)
said the knob controls how long the recycle waits "before forcing the
rebuild" and that 0 means "never forces". That is backwards: nothing
is ever forced through. On timeout the wait gives up, increments the
starvation counter, logs a warning, and stays armed -- the dictionary
threshold is the only actual backstop. Rewrite all three to state the
real policy: once the armed window exceeds the knob, the NEXT
table(...) call may block the calling thread for up to the knob's
value waiting for the backlog to drain; 0 disables blocking entirely.

WARN log wording "re-arming opportunistically" -> "staying armed":
the arm is never consumed on a timeout, so nothing re-arms.

Test fixes in SymbolDictRecycleStarvationTest:
- testTimeoutLogsAndReArms: the "second table() must not re-block"
  probe was placed after a row had been queued (pendingRowCount==1),
  so it short-circuited on maybeRecycleForDictReset()'s precondition
  guard before ever reaching the wait -- vacuously true regardless of
  starvationWaitDoneThisArm. Moved the probe earlier, to a table()
  call with pendingRowCount==0, so it actually exercises the "at most
  one blocking wait per armed window" guard.
- testBlocksThenRecyclesWhenAcksArrive: raised maxWaitMillis from 400
  to 700ms (keeping the 150ms release delay) so the full recycle path
  (I/O loop join, engine close, rebuild, fresh handshake) has real
  headroom under the elapsedMs<maxWaitMillis assertion instead of ~250ms.
- Both testBlocksThenRecyclesWhenAcksArrive and
  testLatchedErrorDuringWaitThrows now join their helper thread
  (releaser/poisoner) in a finally block, so an assertion failure
  can no longer leak a non-daemon thread.

Re-ran SymbolDictRecycleStarvationTest (5/5) and the full
SymbolDictRecycle* battery (27/27), all green.
Adds SymbolDictRecycleOutageTest covering two interleavings between the
symbol-dictionary recycle swap and events outside the producer's own
control: a real connection outage on its own stream (recycle triggers
while the pre-recycle I/O thread is mid-reconnect against a killed
server; step 2's close() joins it, step 7's fresh connect recovers once
the endpoint accepts again), and a sibling orphan drainer mid-drain
(the recycle only tears down the foreground sender's own cursor
engine/I/O loop, leaving a concurrently-gated BackgroundDrainer
untouched and able to complete afterward).

The third scenario from the task brief -- OrphanScanner.isCandidateOrphan
rejecting an empty slot directory -- turned out to already be pinned by
OrphanScannerTest#testIsCandidateOrphanDirect and
#testEmptySlotDirIsNotAnOrphan, so no new test was added for it.

Test-only change; no production code touched.
SymbolDictRecycleCatchUpSkipTest pins that recycleForDictReset()'s step 7
reconnect never pays for a delta-dictionary catch-up frame: the rebuilt
engine sits on a freshly-emptied slot, so the new loop's sentDictCount
mirror seeds from PersistedSymbolDict.recoveredSize() == 0 and
setWireBaselineWithCatchUp's gate stays false for the whole first
post-recycle connection.

The core scenario chains the negative and positive observations in one
test so the zero count is provably a property, not a handler blind spot:
after the recycle sends zero zero-table frames and tiles ids from 0, the
handler force-drops the connection, and the resulting UNPLANNED reconnect
does catch up -- bounded to exactly the new epoch's symbols, never
replaying the retired epoch's. A follow-up symbol then ships with a delta
start above 0, pinning that resetSymbolDictStateForNewConnection() on the
plain-reconnect path preserves sentMaxSymbolId rather than folding the
recycle's baseline reset into itself.

A second test repeats the zero-catch-up observation under
initial_connect_retry=async, where step 7's reconnect funnels through
ensureConnected()'s ASYNC arm and the handshake completes on the I/O
thread.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K2Hw6TfgX9sBB2L8oSdaWn
rev-t10 traced resetSymbolDictStateForNewConnection() to its single call
site in ensureConnected(), which the unplanned I/O-thread reconnect
(swapClient) never reaches -- so the old comment credited a function that
does not even run on this path. sentMaxSymbolId survives the plain
reconnect because nothing touches it there; only recycleForDictReset()'s
step 5 ever zeroes the baseline. The class javadoc's failure-mode
attribution carried the same imprecision and is reworded to match. The
assertion itself was traced sound (symbolDeltaBaseline() ->
encoder.beginMessage -> deltaStart) and is unchanged; comment-only diff.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K2Hw6TfgX9sBB2L8oSdaWn
Adds SymbolDictRecycleCrashWindowsTest, pinning what a restarted sender
recovers if the process crashes at each of four points around
QwpWebSocketSender.recycleForDictReset()'s 8-step symbol-dictionary
recycle swap:

- (a) before step 2 (the barrier that starts the swap): the pre-recycle
  epoch's slot holds a fully-acked batch on disk. Recovery must find
  that residue, recognize it as already acked (nothing to replay) and
  resume the SAME dictionary rather than starting fresh. Constructed by
  closing fast against a server that never acks (so the fully-drained
  unlink never fires) and then stamping the ack watermark directly to
  declare the batch acked retroactively, mirroring
  DeltaDictRecoveryTest#writeAckWatermark.

- (b) between step 3 (fully-drained close of the old engine) and step 6
  (rebuild): the slot is empty. Constructed by driving a real recycle to
  completion and then closing immediately, before any flush touches the
  freshly-rebuilt engine -- finishClose treats "nothing published yet"
  as fully drained too, so this unlinks everything step 6 just created,
  leaving the same empty state step 3 alone would have left.

- (c) after step 7 (reconnect), before the new epoch's first flush: the
  slot holds a freshly-rebuilt engine's own state files but no data.
  Constructed by snapshotting the rebuilt slot's bytes before closing
  (there is no supported way to release just the slot's OS flock without
  running finishClose's unlink), closing for real so nothing leaks, then
  restoring the snapshot on top of the vacated directory.

- (d) an ordinary mid-operation crash one epoch into the post-recycle
  steady state, to prove the epoch swap does not corrupt normal backlog
  recovery. Uses the same close-fast-against-a-non-acking-server idiom
  as RecoveryReplayTest, but only after the recycle's fresh connection
  is established.

Arms (b) and (c) both replay nothing and both look "empty" at first
glance, but they are not the same recoverable state and a restarted
engine can tell them apart: (c)'s slot carries a manifest with collapsed
boundaries alongside a same-based, zero-frame active segment, which
SegmentRing.recover()'s chain-building accepts as a RECOVERED (if empty)
chain, while (b)'s slot carries no engine state at all and recovers as
EMPTY. wasRecoveredFromDisk() is the pinned, distinguishing observable
between the two, asserted explicitly instead of writing two
assertion-for-assertion duplicate tests.

Every arm's oracle: the recovered sender keeps ingesting; the symbols it
and its predecessor registered are exactly and correctly reconstructable
from the wire (each fresh server handler rebuilds the per-connection
delta dictionary); and no data frame is delivered more than the
at-least-once contract allows (each handler counts data frames so a
spurious re-send would show up as an unexpected count).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K2Hw6TfgX9sBB2L8oSdaWn
Applies review findings 1, 2, 3, 4, 6, 7, 8 and 10 from the task-11
review (findings 5, 9, 11 are deferred to the whole-branch review):

- Arm (b) never actually pinned the "empty slot" disk image it exists
  to test -- it inferred emptiness from wasRecoveredFromDisk()==false
  on the successor instead of asserting the crashed sender's own slot
  dir. Adds an explicit listDir(slot) == [".lock", ".lock.pid"]
  assertion right after the crashed sender closes (symmetric to arm
  (c)'s pre-existing file-set assertion), plus a second one after the
  successor's own fully-drained close, proving the empty-slot state is
  stable rather than a one-shot coincidence. This also replaces arm
  (b)'s closing assertion, whose message previously credited the
  successor's "recovery" for cleaning up a stale manifest that the
  crashed sender's own close had already removed.

- Arm (c) snapshotted the freshly-rebuilt slot before waiting for the
  manager worker's asynchronous hot-spare provisioning to settle, so a
  mid-provision snapshot could race-capture a zero-magic spare that
  recovery would hard-fail on. Adds a bounded poll
  (awaitExactFileSet) before the snapshot, reusing the same expected
  file list (now a shared FRESH_REBUILD_FILES constant) as the
  post-restore assertion so the two can never drift apart.

- Arms (a) and (d) loosely asserted recoveredMaxSymbolId() >= 1, which
  a leaked epoch-0-plus-epoch-1 dictionary would also satisfy. Tightens
  both to the deterministic exact value (1L), keeping their explanatory
  messages as-is now that the assertion actually proves what the
  message claims.

- Widens QwpWireTestUtils.tableCount to public (its sibling frame
  helpers already are) and deletes this suite's local copy plus its
  now-unnecessary justification javadoc.

- Inlines the AckAllHandler bindings in arms (b) and (c) that were
  never read (dict()/dataFrameCount() were only meaningful on the
  "fresh" server's handler, not the "crashed" one).

- Class javadoc: notes that arm (c)'s snapshot/restore does not cover
  the logical slot lock (lives outside the slot dir; sender.close()
  reclaims it, acquireLogical recreates it -- benign), and corrects
  three mechanism imprecisions the review traced: the "never published"
  fully-drained check lives in close(boolean), not finishClose; a
  fully-drained close does not remove .lock/.lock.pid; and arm (b)'s
  SegmentRing.recover() counterpart is the no-manifest fall-through to
  Recovery.empty(), not the manifest-present collapse branch.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K2Hw6TfgX9sBB2L8oSdaWn
A sender that degrades to full self-sufficient frames after a symbol-dict
persistence fault (disableDeltaDict) used to stay degraded for the rest of
its life -- deltaDictEnabled was set once at engine construction and never
re-evaluated. The symbol-dictionary recycle already rebuilds the cursor
engine from scratch on every swap; make that rebuild also re-derive
deltaDictEnabled from the fresh engine instead of carrying the old one's
verdict forward. If the underlying fault has cleared, the next recycle
heals the sender back into delta mode; if it has not, the fresh engine
degrades again on its own first append, the same ordinary catchable
LineSenderException as any other persistence fault -- a degrade, never a
latched recycleFailure terminal state.

Promote the epoch and starvation-timeout counters from @testonly
accessors to permanent public API: getSymbolDictEpochForTest() becomes
getSymbolDictEpoch(), and getSymbolDictResetStarvationTimeoutsForTest()
becomes getSymbolDictResetStarvationTimeouts(). Both counters already
existed; this only changes their visibility and documents their
thread-safety contract (producer-thread-written, so a read from another
thread is an eventually-consistent snapshot). Add a third counter,
getSymbolDictResetsPerformed(), incremented alongside the epoch inside
recycleForDictReset() -- the two move together today but are defined and
incremented independently, since a future change could roll the epoch by
some path other than a completed recycle swap.

Migrate every existing call site of the two renamed getters (7 test
files, 48 + 15 occurrences) to the new public names.

SymbolDictRecycleHealingTest covers all of this: a fault-then-heal
recycle proves deltaDictEnabled and wire framing both return to proper
delta encoding (the second post-recycle frame's delta starts where the
first left off and carries only the newly-added symbol, the shape only
delta mode produces); a fault-persists variant proves the fresh engine
degrades again without escaping as a raw error or latching the sender
terminal; and a two-recycle run asserts the three metrics getters track
correctly in lockstep while the starvation counter stays untouched.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K2Hw6TfgX9sBB2L8oSdaWn
Applies task-12-review.md findings Q1-Q6.

Q1: the three symbol-dictionary-recycle metrics fields (symbolDictEpoch,
symbolDictResetsPerformed, symbolDictResetStarvationTimeouts) become
volatile -- they are permanent public API now, and their obvious reader
is a monitoring thread on some other thread, unlike every other
symbol-dictionary-recycle field they were modelled on. A plain,
non-volatile long gives a cross-thread reader no visibility guarantee at
all under the JMM (a polling loop can legally observe 0 forever), whereas
the javadoc claimed an "eventually-consistent snapshot". Reworded the
three javadocs to state the guarantee volatile actually gives: each read
sees the latest write the producer thread completed, with no atomicity
across the three counters -- a concurrent reader can see the epoch
already advanced while resets-performed still reflects the prior value,
even though the producer thread writes them on adjacent lines.

Q3: getSymbolDictResetStarvationTimeouts()'s javadoc named
recycleForDictReset() as the writer by importing getSymbolDictEpoch()'s
caveat sentence verbatim; the starvation counter is actually written in
maybeBlockForStarvedReset(). Named that method directly instead.

Q4: getSymbolDictResetsPerformed() said swaps this sender has
"completed" without stating that the counter advances at step 5, before
the engine rebuild (step 6) and reconnect (step 7) -- so a recycle that
later latches recycleFailure at step 6/7 still counts. Made that
explicit, mirroring the precision getSymbolDictEpoch() already had.

Q2: SymbolDictRecycleHealingTest's healing test asserted
isDeltaDictEnabledForTest() right after the recycle and attributed the
result to the healed facade, but the same assertion holds unconditionally
-- a fresh engine's construction never touches mmap, so it reports true
whether or not the facade was healed (the persistent-fault sibling test
proves this directly). Reworded the message to state what that assertion
actually pins, and added the discriminating check: re-assert after the
first post-recycle flush, the fresh engine's first real append -- a
still-armed facade would degrade it there, so staying true is real
evidence of healing.

Q5: AckAllHandler never reset its ack sequence per connection, unlike
CapturingAckHandler right below it in the same file. A rebuilt engine
restarts its raw FSNs at 0 after a recycle, so the stale, unreset
sequence could satisfy awaitAckedFsn with an ack for a frame that was
never published on the new connection -- a weaker gate than it looks,
even though the specific sequences in these tests happened not to
collide. Reset nextSeq on every new connection, matching the sibling
handler and existing repo precedent for connection-aware ack sequencing.

Q6: both fault-injection tests caught LineSenderException with a comment
claiming parity with MmapFaultDegradesTest's guard, but never actually
asserted the message MmapFaultDegradesTest checks
("failed to persist symbol dictionary before publish") -- so the catch
could equally have swallowed an unrelated connection-level exception.
Added the same message assertion to all three catch sites (two in the
persistent-fault test, one in the healing test) so the comment's claim
now holds.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K2Hw6TfgX9sBB2L8oSdaWn
The symbol-dictionary cap error told callers to close the sender and
build a new one, but gave no way to avoid hitting the cap in the first
place. Append a sentence pointing at the automatic dictionary reset
knobs (symbol_dict_reset, symbol_dict_reset_threshold) and the manual
Sender.resetSymbolDictionary() escape hatch, so the error message
matches the recycle feature this client now ships.

Raise QwpConstants.MAX_SYMBOL_DICTIONARY_SIZE from 1_000_000 to
2_000_000 to mirror the server-side constant of the same name, which
moved to 2_000_000 in questdb OSS commit 306062e243 (#7468). That
commit is contained in release tag 10.0.0, the support floor for this
client, so client <= server holds across the whole supported fleet.

Update DeltaDictCeilingTest and GlobalSymbolDictionaryTest, which
pinned the old 1,000,000 value and message text, to the new cap and
add a case that drives the dictionary to the cap with automatic reset
disabled (symbol_dict_reset=off) and a threshold configured at the
cap, confirming the refusal still fires and still names the reset
valve even on a sender that has it switched off.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K2Hw6TfgX9sBB2L8oSdaWn
The new cap-reached-while-armed test never calls sender.flush(): the
2M fill goes through the raw GlobalSymbolDictionary test accessor, and
the one Sender-routed call throws inside symbol() before a row
completes. armIfEligible() only runs from the tail of a completed
flush(), so it never had a code path available to flip isResetArmed()
to true regardless of whether symbol_dict_reset was on or off -- the
two assertFalse(ws.isResetArmed()) checks passed identically either
way and proved nothing about the knob under test.

Drop both assertions and note in the test's javadoc that arming
semantics are out of scope here and are pinned instead by
SymbolDictRecycleArmingTest.testArmsAtThreshold, which drives real
rows through flush() and is the idiom that actually exercises
armIfEligible(). The test's brief-mandated assertion -- the cap
refusal still fires, with the new reset-valve message, when reset is
disabled -- is untouched and remains the load-bearing check here.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K2Hw6TfgX9sBB2L8oSdaWn
recycleForDictReset() no longer latches the sender terminal when the
recycle's reconnect fails. Step 7 moves out of the latching try block:
steps 1-6 still latch recycleFailure (a half-swapped sender genuinely
cannot make progress), but a failed ensureConnected() logs a warning and
rethrows to the triggering caller without latching. By step 7 the swap has
committed, so the sender is coherent - connected == false, loop and client
already closed and nulled by ensureConnected's own catch, the fresh engine
attached, the step-5 epoch and swap counters correctly left incremented -
and the ordinary sendRow() -> ensureConnected() path retries the connect,
and only the connect, on the next send. Nothing re-runs a teardown step,
and nothing can fire a second swap meanwhile: the fresh dictionary sits
below the threshold, manualResetRequested was consumed at step 5, and
maybeRecycleForDictReset requires connected.

This removes a default-configuration brick. A sender built with no
reconnect_* knob resolves initialConnectMode to OFF, so step 7 is a
single-shot connect; a server restart or an LB blip across a drained,
armed sender therefore latched every later table()/flush() call forever,
including seconds later once the endpoint was back.

Deferring the connect to the next send exposed a second defect, which this
commit also fixes. ensureConnected() calls
resetSymbolDictStateForNewConnection(), which cleared
currentBatchMaxSymbolId unconditionally. That watermark is batch-scoped,
not connection-scoped - a flush ships exactly [sentMaxSymbolId+1 ..
currentBatchMaxSymbolId] - and clearing it was harmless only because
build() connects before the application can register a symbol. On the
deferred path symbol() runs first, so the clear made the next flush ship
an empty delta while its rows referenced symbol id 0: rows on the wire
pointing at ids the server never received. The reset now runs only from
the drained state the old code assumed (no pending rows, no row in
progress).

testDefaultConfigRecycleSurvivesFailedReconnect pins both. It drives a
default-config sender (no reconnect knobs), kills the listener at a
drained instant, asserts the recycle's throw reaches the caller, asserts
recycleFailure is not latched by ingesting successfully once the endpoint
returns on the same port, and asserts the recovered stream defines every
symbol its rows reference. Against the pre-fix code it fails on the latch;
with the latch fixed but the watermark clear restored it fails on the
empty dictionary.

Documentation and hygiene alongside:

- getTotalFramesReplayed/getTotalFramesSent/getTotalReconnectAttempts/
  getTotalReconnectsSucceeded/getTotalServerErrors now state that they
  read the live send loop and therefore restart at 0 on every recycle
  ("since the last recycle"), and point at the lifetime-scoped
  getSymbolDictEpoch/getSymbolDictResetsPerformed for correlation.
- Sender.resetSymbolDictionary(), its QwpWebSocketSender override and
  LineSenderBuilder.symbolDictReset() now say that the manual valve is a
  permanent no-op while symbol_dict_reset is off, because armIfEligible
  gates on that knob.
- armIfEligible's javadoc names both call sites; resetSymbolDictionary()
  calls it too, and the stale single-call-site claim invited a wrong
  inlining refactor.
- The builder's symbolDictReset default references
  DEFAULT_SYMBOL_DICT_RESET_ENABLED instead of a hardcoded true.
- SymbolDictRecycleOutageTest joins its trigger thread in a finally so an
  assert failure cannot leave a thread inside the sender, and records why
  the revived server's handshake count stays >= 1 rather than == 1.
- SymbolDictRecycleCrashWindowsTest inlines an unread handler binding, and
  SymbolDictRecycleHealingTest's AckAllHandler carries a warning that its
  per-connection ack reset assumes every connection change is a recycle
  and must not be copied into a plain-reconnect test.

Client suite: 3045 run, 0 failures, 0 errors, 2 skipped.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K2Hw6TfgX9sBB2L8oSdaWn
Review C1: recycleForDictReset step 3 re-acquired the slot flock without
waiting for a deferred close to release it. When the SF worker is wedged
in a syscall past SegmentManager's bounded join, CursorSendEngine.close()
returns with the flock retained and isCloseCompleted() false, releasing
both from the worker's exit path; the step-6 rebuild then threw
SlotLockContentionException on the retained flock and latched the sender
permanently terminal -- converting exactly the transient disk stall the
deferred-close machinery exists to survive into a hard sender death on
the default (SF + recycle-on) path.

recycleForDictReset now mirrors close()'s deferred-close discipline:
awaitDeferredEngineClose parks (awaitAckedFsn-shaped) until the deferred
cleanup confirms the flock release, re-arming the shared flock-release
retry driver each pass like isSlotLockReleased() does. Only exhausting
the 30 s budget -- a genuinely dead worker -- latches terminal, and that
path hands the still-locked engine to retainedEngine so a pool re-probe
recovers the slot's capacity if the worker ever exits; close() no longer
clobbers slotLockReleased to true while such an engine is pending.
SymbolDictRecycleDeferredCloseTest pins both branches with a wedged-
worker harness (red-proofed: without the await, the survival test dies
with the exact SlotLockContentionException the review traced).

Review Mo1: getAckedFsn and awaitAckedFsn snapshot cursorEngine into a
local (the recycle transitions it non-null -> null -> non-null), and
while it is null they report the durable watermark the recycle barrier
proved (new lastRecycleDurableFsn) instead of collapsing to -1.

Review Mo2: PooledSender forwards resetSymbolDictionary() to the live
delegate instead of inheriting the interface's no-op default; pinned by
a pooled-path test.

Review Mo5 (verified: released 9.4.x servers cap the dictionary at 1M):
document the pre-10.0.0 compatibility constraint on the 2M client cap at
QwpConstants.MAX_SYMBOL_DICTIONARY_SIZE and symbolDictReset(boolean).

Review mi7/mi8: step 6 refuses a rebuilt engine that recovered from disk
(the empties-the-slot contract was breached) and resets slotLockReleased
after a successful swap. Review mi3: recycle javadoc now says seven
steps / steps 2-6, the resetArmed and resetSymbolDictionary docs match
the code, and EngineRebuildFactory moves out of the field block.

Review Mo4: SymbolDictRecycleFsnContinuityTest wraps every test in
assertMemoryLeak, and LineSenderBuilderWebSocketTest pins the config
boundaries: threshold == 2M accepted on both config paths, the
symbol_dict_reset=on parse branch, the invalid-value message, and the
three fluent-setter transport guards.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@mtopolnik

Copy link
Copy Markdown
Contributor

[PR Coverage check]

😍 pass : 251 / 271 (92.62%)

file detail

path covered line new line coverage
🔵 io/questdb/client/Sender.java 70 85 82.35%
🔵 io/questdb/client/cutlass/qwp/client/QwpWebSocketSender.java 162 167 97.01%
🔵 io/questdb/client/impl/PooledSender.java 2 2 100.00%
🔵 io/questdb/client/impl/ConfigSchema.java 3 3 100.00%
🔵 io/questdb/client/cutlass/qwp/client/sf/cursor/CursorWebSocketSendLoop.java 14 14 100.00%

jovfer added 2 commits August 20, 2026 16:04
getAckedFsn()/awaitAckedFsn() read cursorEngine, fsnEpochBase and
lastRecycleDurableFsn from monitoring threads while the recycle
reassigns all three on the producer thread. The sibling observability
counters went volatile in an earlier round for exactly this reader;
these three carry the same contract, so a monitor could observe a
fresh engine with a stale epoch base and report an FSN dip to -1.
Also document resetSymbolDictionary() as producer-thread-only: it
mutates unsynchronized producer state, and the fire-and-forget javadoc
framing invited cross-thread calls (notably via PooledSender).
Red first: the outage tests now assert the store-and-forward contract
(producer never sees a transport error nor a reconnect budget after the
initial connect) instead of pinning the step-7 foreground connect they
used to. The production change lands in the next commit.
jovfer and others added 30 commits August 27, 2026 02:56
Review-round and finding identifiers resolve to nothing once the PR
is squashed; describe the behaviour instead. Also drop a close() that
the try-with-resources already performs.
The builder's rebuild factory is the half that forwards the live error
handler into constructEngineOnSlot -> quarantineTornSlot, and that
hand-off is only observable when a rebuild really has a slot to set
aside. The new test plants SegmentSkipQuarantineTest's tainted slot
(several never-acked segments, the oldest one's magic overwritten) into
the empty slot the recycle's step-3 close just left behind, then lets the
real factory run: the recycle still commits, and the handler installed
after build() receives the DATA_LOSS SenderError naming where the bytes
were set aside. The sender is built without an error handler at all, so a
regression that forwarded the build-time handler instead of the live one
leaves the notification nowhere -- verified by mutating the delegation to
the handlerless overload, which fails the new assertion.

Also align the test javadocs with the recycle step numbering and drop the
last review-era names.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ru8dSnpZsgkg3uPkfwyGvp
A monitor thread parked between the accessor's two volatile reads across
a real recycle observes the (old base, fresh engine) pair; the clamp
against the durable watermark keeps it from reading -1. The deferred-close
park witness now also samples both accessors while the engine is null.
Both assertions go red with the clamp or the durable write removed.
A resume while the SF worker is still wedged used to recompute the
deadline and park the producer for a fresh full budget on every
table()/sendRow(). The deadline now lives in a field for the pending
close: later entries probe the close and rethrow without parking, and a
completed await resets it so the heal path's second close gets its own
budget. Pinned by timing the retried call.
isSlotLockReleased() re-probing a retained outgoing engine latches true;
the recycle cleared it only at the commit, so the flag stayed true across
the rebuild and a possible heal round-trip while the rebuilt engine
already held the flock. The write now follows the await; close() derives
the flag itself when a rebuild fails, so a pool never loses the slot.
The fresh dictionary is allocated before the rebuild so the commit block
cannot throw and leave a flock-holding engine unassigned.
After a recycle the close-drain timeout printed raw engine FSNs while
flushAndGetSequence() hands out epoch-rebased ones, so the printed target
fed back into awaitAckedFsn() answered a different question. The WARN,
the thrown message and the loop's two replay log lines now use the
external scale, matching the SenderError spans. Pinned after a real
recycle.
symbol_dict_reset_threshold now stops at half the protocol cap, the
re-arm floor's own ceiling: arming happens at a flush tail and the swap at
the next drained table(), so a threshold at the cap tripped the cap error
before any recycle could run. symbol_dict_reset_max_wait_millis rejects
values whose nanosecond conversion overflows, which used to invert the
knob into never blocking.
symbol_dict_reset_threshold now caps at half of
MAX_SYMBOL_DICTIONARY_SIZE, so the fixture that configured it at the
old full-cap value tripped the builder's own validator instead of
reaching the dictionary-fill assertions it exists to cover. Point the
fixture at the threshold's own new cap and reword the javadoc to
name the two caps separately.
A recycle's reconnect fires RECONNECTED with no DISCONNECTED before it
(pinned); a rebuild-time quarantine is delivered synchronously on the
producer thread like the build-time one; a fresh loop on an engine
recovered at build() seeds its mirror from the recovered dictionary, not
from zero. setEngineRebuildFactory is documented and refuses a closed
sender; getTotalAcks and getTotalBackpressureStalls say they restart on
every recycle like their siblings.
The epoch counter and the completed-swaps counter were incremented on
adjacent lines by the only code path that touches either, so they could
never differ; the getter and the comments explaining a divergence the
code cannot produce are gone. The epoch is the lifetime-scoped counter
the per-recycle getters point at.
getSymbolDictEpoch()'s javadoc still said "three" symbol-dictionary-
recycle counters. Only two remain after the swap counter was
dropped: the epoch itself and the starvation-timeout counter.
Step 6 of recycleForDictReset()'s javadoc still said the epoch
counter and a "completed-swap counter" both advanced. Only the
epoch counter advances now. Fixed the same stale plural in three
sibling spots: completeRecycleRebuild()'s summary, the swap's
parenthetical (dictionary, counters, epoch), and recycleFailure's
javadoc.
The interrupted-recycle test asserts the abandon branch instead of
gating on it; the step-7 fault test no longer calls its own red-proof
untested; the starvation suite carries timeouts and wall-clock margins
that survive a busy runner; the crash-window suite quotes the recycle's
actual seven steps; planning-document references are gone from test
javadocs; the two FSN seams follow the ForTesting convention; one licence
header matches its siblings.
SymbolDictRecycleFsnContinuityTest's class javadoc still named
Task 5 as the source of the engine-rebuild behavior it pins.
Reworded to describe the recycle itself, matching the planning-
reference cleanup already done across the rest of this suite.
The full 3068-test client suite failed once at the hard assertion
Step 1 of this task added: under load, an interrupted producer's
loop close can complete normally instead of throwing, because
CountDownLatch.await() can observe the shutdown latch already at
zero by the time it re-checks after the interrupted exception, so
close() proceeds instead of failing loud. The class alone (13/13)
and a dedicated 20-iteration run both missed this race.

The recovery half (never terminal, finishes the recycle on later
sends) now runs unconditionally regardless of whether the loop
close threw. The abandon-specific assertion is gated behind
Assume.assumeTrue: a lost race reports the test as SKIPPED with an
explanatory message instead of silently passing or intermittently
failing. The re-asserted-interrupt-flag assertion still runs
whenever the throw happened. Javadoc and inline comments updated
to describe both outcomes instead of claiming the throw is
deterministic.
The abandon WARN in drainOnClose now prints epoch-rebased FSNs
like the timeout path does, instead of the raw ring scale.
The epoch javadoc's atomicity caveat is now stated once instead
of twice. The rebuild-factory setter's javadoc no longer claims
that null cancels a recycle that is already armed or pending; it
only stops future arming. The arming javadoc names the epoch
counter instead of a nonexistent resets counter.
# Conflicts:
#	core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpWebSocketSender.java
#	core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/BackgroundDrainer.java
#	core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/CursorWebSocketSendLoop.java
The merge conflict resolution had retyped the pre-existing
connect(..., String authorizationHeader, ..., maxFrameRejections,
poisonMinEscalationWindowMillis, catchUpCapGapMinEscalationWindowMillis)
overload in place by appending the three symbol-dict-reset params,
instead of adding a new overload alongside it. That breaks binary
compatibility for callers compiled against the original 23-arg
signature (NoSuchMethodError), which is exactly what
ExportedApiCompatibilityTest.testPreBranchQwpWebSocketSenderConnectOverloadsStillLink
caught.

Restore the original 23-arg overload (forwarding to
connectWithCredentialSupplier with the DEFAULT_SYMBOL_DICT_RESET_*
constants) and keep the wider 26-arg overload as a separate,
additional overload for callers that need to configure the recycle
knobs explicitly on the String-credential path.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01324EkBQCNJWuA3LFDBJZtF
DEFAULT_SYMBOL_DICT_RESET_MAX_WAIT_MILLIS drops from 30_000 to 0: the
recycle now runs only when a table() call finds the backlog already
drained, and never blocks the producing thread. A positive value stays
available as an explicit opt-in with unchanged semantics (one bounded
wait per armed window). The builder javadoc states the trade plainly
and names the monitoring signal for a recycle that never finds its
drained instant: isResetArmed() staying true while getSymbolDictEpoch()
does not advance.

This restores the reconnect-reset design's posture (spec 13 s11.4): the
bounded wait on the hot path was plan-added and its default was never
signed off.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01324EkBQCNJWuA3LFDBJZtF
maybeBlockForStarvedReset() and awaitDeferredEngineClose() park with
LockSupport.parkNanos, which returns immediately while the caller's
interrupt flag is set -- the wait degenerated into a busy spin, and a
flag that survived to the drained exit made the swap's loop-close join
throw, manufacturing a CLOSE_LOOP abandon out of a successful wait.

Both loops now clear the flag per park iteration, restore it on the
timeout/throw exits, and swallow it on the success exit, the same
policy CursorSendEngine's flock-release retry driver uses. Two tests
pin the policy: the drained exit must complete the recycle with no
abandon and no leftover flag; the timeout exit must restore the flag
after waiting out its full budget.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01324EkBQCNJWuA3LFDBJZtF
awaitDeferredEngineClose() only restored a cleared interrupt flag
inline, right before its own deadline throw. A stray RuntimeException
or Error from isCloseCompleted() or ensureFlockReleaseRetryScheduled()
after the flag was cleared would propagate around that inline restore
and silently drop it -- the asymmetry maybeBlockForStarvedReset()'s
wrap-the-whole-loop shape was designed to avoid.

Wrap the loop in the same try/catch(Error)/catch(RuntimeException)
shape as its sibling, restoring in both catch blocks and dropping the
now-redundant inline restore before the deadline throw (that throw is
a LineSenderException, itself a RuntimeException, so the catch below
already covers it) -- every throw exit now restores exactly once, the
completed exit still swallows.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01324EkBQCNJWuA3LFDBJZtF
An abandoned recycle's CLOSE_LOOP resume used to drop sentMaxSymbolId
with the dead loop's mirror, so in delta mode the next flush re-encoded
the whole dictionary as one delta -- and a dictionary past the server
batch cap could never flush again (the chunk fallback is full-dict
only). The resume now re-registers [0..sentMaxSymbolId] as deferred
dictionary chunks plus their closing commit on the ring; the fresh loop
replays them ahead of any data frame, so the baseline survives.

Any publish failure degrades to the old baseline drop, and the catch
closes the chunks' deferred-commit debt itself: a commit failure is not
covered by publishDictionaryChunks' internal orphan handling, and an
open group would clamp ackedFsn for the connection's life. The
publishDictionaryChunks assert now admits the delta-mode caller whose
range the write-ahead persist made durable, and still rejects a
delta-mode caller in any other state; memory mode (no persisted dict)
and the degraded delta-to-full state stay accepted.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01324EkBQCNJWuA3LFDBJZtF
The CLOSE_LOOP resume's re-registration Error arm dropped the baseline
and rethrew on the assumption that publishDictionaryChunks always
closes its own chunk debt before an Error escapes it. That holds only
for an Error thrown INSIDE publishDictionaryChunks. An Error from the
resumeCommitFaultForTesting seam or from sendCommitMessage() lands in
this arm AFTER publishDictionaryChunks already returned successfully,
having published chunks and set hasDeferredMessages -- so the debt was
left open, and an open deferred group clamps ackedFsn for the
connection's whole life.

The Error arm now closes that debt the same way the Throwable arm
does: guarded by hasDeferredMessages, via the best-effort
commitOrphanedDictionaryChunks(e), before dropping the baseline and
rethrowing. The Error itself is never swallowed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01324EkBQCNJWuA3LFDBJZtF
A fault injected between the resume's chunk publish and its commit must
degrade to the shipped behaviour (baseline dropped, resume state
cleared, next send works) and must leave no open deferred-commit group
behind. The test pins both, and documents explicitly that the fallback
does not heal the over-cap population -- it only refuses to wedge.

A second test pins the Error arm: the Error propagates, yet the debt is
closed and the baseline dropped.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01324EkBQCNJWuA3LFDBJZtF
A process death between the CLOSE_LOOP resume and the reconnect leaves
the re-registration chunks, their commit, and the data frame on the SF
ring. A fresh sender on the same slot replays them whole against a
server that models the real deferred-ack contract (no ack for deferred
frames until the group's commit): the drain completes and the replayed
dictionary covers the old ids ahead of the data frames.

The test also asserts hasDeferredMessagesForTesting()==false
synchronously right after the resume runs, before the crash-simulated
close. This is necessary, not decorative: the crash-replay outcome
(drain + dictionary) alone cannot pin whether the resume itself closed
its own deferred-commit debt, because two independent, pre-existing
mechanisms backstop it regardless -- close()'s own redundant
"!deferCommit && hasDeferredMessages" safety net (which still runs
under close_flush_timeout_millis=0, since that only gates the
drain-wait, not the local flush/commit/seal steps) and the recovered
slot's own reconnect dictionary catch-up (CursorWebSocketSendLoop's
sentDictCount/hasReplayDictionaryDependency mirror, seeded from the
persisted .symbol-dict on every delta-mode recovery). Verified by
mutation: deleting resumeRecycleIfPending's sendCommitMessage() call
leaves drain()/dict() green, but the synchronous check catches it
immediately.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01324EkBQCNJWuA3LFDBJZtF
The crash-replay test's javadoc claimed the deferred-ack server "acks
everything only because the commit frame is there to close it" --
implying the drain + dict-order outcome isolates the resume's own
commit. It does not: close()'s own redundant commit-closing safety net
and the recovered slot's reconnect dictionary catch-up both produce
the same outcome even with the resume's commit missing entirely, as
the inline comment beside the synchronous assertion already states.
Rewrite the javadoc to say plainly what drain + dict-order actually
proves (SF-ring persistence and in-order replay against a strict
deferred-ack-withholding server) and name the synchronous
hasDeferredMessagesForTesting() assertion as what isolates the
resume's own commit.

Also: update the suite's class-level javadoc from "four points" to
"five points" with an (e) entry for the new test, and document
DeferAwareCaptureHandler's single-connection assumption explicitly (no
per-connection dict reset, so phase 2 must never reconnect against it).

Documentation only; no test logic changed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01324EkBQCNJWuA3LFDBJZtF
resetSymbolDictionary(), the dictionary-cap error message, and
armIfEligible() now all say the same thing in one sentence each: the
recycle runs at a table() call that finds the backlog acknowledged,
and table() is the only trigger point. Sender.table() documents
table-per-row as the row API's contract, so no per-row branch is added
to the hot path for callers who hold one table open forever.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01324EkBQCNJWuA3LFDBJZtF
The recycle swap seeds the fresh I/O loop with markEverConnected(), so
an endpoint-policy rejection (401) on the post-recycle handshake stays
transient: the sender keeps buffering and the rows land once the
endpoint relents. Deleting the seed makes this test observe the
latched terminal, which is exactly the data-loss regression it guards.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01324EkBQCNJWuA3LFDBJZtF
Two real recycles with different per-epoch frame counts assert the
exact base after each swap: (L1+1) after the first, (L1+1)+(L2+1)
after the second, and the next published FSN lands exactly on the
accumulated base. A base that is re-assigned instead of accumulated
passes every monotonicity check in the existing suite and fails only
these equalities -- the second recycle is the discriminator.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01324EkBQCNJWuA3LFDBJZtF
- Drop core/.factorypath from the index and ignore it alongside its
  .classpath/.project siblings.
- Make resetArmed volatile: isResetArmed() is a documented monitoring
  surface, matching the file's other volatile monitoring fields.
- Document that getSymbolDictResetStarvationTimeouts() counts only the
  opt-in bounded wait, so it stays structurally 0 at the default
  symbol_dict_reset_max_wait_millis=0 -- 0 does not mean "no starvation".
- Add the deferral caveat to both public entry points of the
  symbol_dict_reset knob (Sender#symbolDictReset and
  DEFAULT_SYMBOL_DICT_RESET_ENABLED): the recycle runs at a table() call
  that finds the backlog acknowledged, so it may be deferred indefinitely
  under sustained load.
- Explain why the CLOSE_LOOP resume's baseline drop stays safe now that
  chunks are ringed before the drop, in both failure arms.
- Rework publishDictionaryChunks' All-or-nothing javadoc for its two
  callers (the full-dict fallback and the delta-mode resume).
- Snapshot the volatile serverMaxBatchSize cap once in the CLOSE_LOOP
  resume, matching the file's snapshot-once convention elsewhere.
- Reword armIfEligible's javadoc: arming is "triggered" (not "consumed")
  from table(), and note a pending resume can still complete a recycle
  from a later flush()-driven ensureConnected().
- Restructure the resetMaxWaitMillis field comment to lead with the 0
  default (opportunistic-only) before describing the opt-in bounded wait.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01324EkBQCNJWuA3LFDBJZtF
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request tandem

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants